Integration dev - #10
Merged
Merged
Conversation
…icate bootstrap_clusters ran all downsampling replicates sequentially in one job, and its fixed runtime (not scaled by attempt) was timing out on large Seurat objects that used to work fine before other refactoring. Split the rule into downsample_cluster_replicate (one SLURM job per downsample_target/replicate pair) and downsample_clusters (a cheap gather step that concatenates the per-replicate TSVs into the final summary table). Verified the new pipeline reproduces the existing reference outputs byte-for-byte and the downsampling test suite passes.
240 minutes was an unjustified guess. A single replicate only downsamples barcodes and reruns clustering, which takes a few minutes even on a laptop; the old monolithic job's own numbers (100 replicates serially exceeding 960 min) implied well under 15 min/replicate. Requesting far more runtime than needed also hurts SLURM queue time.
… job per target do.call(SCTransform, sct_args) embedded the full Seurat object as a literal value in the call frame; Seurat's internal Parenting() helper stringifies the entire call stack on each progress tick during ScaleData's regression step, which made that stringification catastrophically slow (40+ min hangs, up to 256GB memory requested on retry) purely as an artifact of using do.call instead of a direct call. Calling SCTransform directly drops real per- replicate runtime to ~2 minutes and ~10-15GB. With replicates that cheap, running each as its own SLURM job meant queue wait time (median ~80min) vastly exceeded actual compute time (~2min) per job. Collapsed the two-rule replicate/aggregate split back into a single downsample_clusters rule per target that loops over all replicates in one process and writes the combined summary table directly, cutting job count for this stage from ~2300 to ~23. Also throttled SLURM job submission/status- check rate in the runner script to avoid overloading the scheduler under heavy job counts. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
temp() on find_markers' output wasn't reliably triggering cleanup across
this workflow's repeated crash/rerun history: several prefixes had already
completed combine_markers successfully yet still had their per-cluster
_markergenes_cluster{N}.csv files left on disk. Delete them explicitly via
input.markers in combine_markers' shell block instead of relying solely on
temp() deletion.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ion tests in CI automatically The existing downsampling tests only ran the rule against a single tiny fixture (300 cells) and asserted correctness, never runtime, so the do.call(SCTransform, ...) performance regression (see the recent downsample_clusters.R/downsample_clusters.smk fix) was invisible to them regardless of whether they ran: a data-size-dependent slowdown has nothing to bite on at a single, tiny scale, and produces byte-identical output. Add a synthetic mid-scale fixture (tests/downsampling/make_medium_fixture.R, 2500 cells via stratified resampling of the existing fixture) and a new test that runs downsample_clusters on both fixture sizes and asserts the wall-clock ratio stays within a generous multiple of the ~8.3x cell-count ratio - loose enough for normal scaling, tight enough to have caught this exact bug. Also discovered that downsample-rule/downsample-workflow (and now downsample-scaling) were gated behind workflow_dispatch and never ran on regular pushes/PRs, which was a second, independent reason this could slip through. All three now run as unconditional jobs. Fixed test_downsampling_snakemake_dry_run.py, which was still asserting on the now-removed downsample_cluster_replicate rule name from the earlier rule-merge and had been silently broken since. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…t_config_validation.py These two assertions were missed when the downsample_cluster_replicate rule was merged into downsample_clusters; unlike the equivalent fix already made in tests/downsampling/test_downsampling_snakemake_dry_run.py, this file runs in the default (non-opt-in) pytest tests -q CI job, so it broke that job on every push since the rule-merge commit (GitHub Actions runs #37, #38, #39). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
actions/checkout@v4, actions/cache@v4, and mamba-org/setup-micromamba@v2 all still declare node20, which GitHub now runs on a forced/deprecated Node 24 runtime with warnings. Bump to actions/checkout@v7, actions/cache@v6, and mamba-org/setup-micromamba@v3, all of which target node24 natively. No breaking changes apply to this workflow's usage (basic checkout, plain path/key/restore-keys caching, environment-file-based micromamba setup). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
integrated downsampling workflow